Summary

This patch updates sequencer's mempool service to execute the same steps when serving an ABCI CheckTx request as it would during regular block building.

Background

The current check-tx implementation was only performing a subset of the checks that would be performed during regular block construction (specifically, during finalize-block). This means that while a check-tx would return a positive response, a subsequent and immediate execution would fail. Furthermore, with the changes in #1318 the distinction between checks and execution has been lifted, moving the current check-tx implementation further away from actual business logic.

With this patch the check-tx logic now uses exactly the same code paths as for regular block construction.

This is done by creating a new App instance with a snapshot that never gets applied to the actual storage.

Changes

• Rename App::execute_transaction to App::deliver_tx: this moves the nomenclature closer to cometbft (and penumbra)
• Introduce App::deliver_tx_bytes as a wrapper to the now private App:deliver_tx.
• Rewrite crate::service::Mempool in terms of App::execute_transaction_bytes
• Move checks for total signed transaction size to App::deliver_tx_bytes
• Change crate::mempool::Mempool to require Arc<SignedTransaction> in its methods: this is a relatively mild change because it wrapped its argument in an Arc anyway. This is useful because App::execute_transaction_bytes returns a Arc<SignedTransacttion>
• Drop metrics we no longer have access to (because transactionq checking now happens in a single opaque method)
• Provide convenience methods to downcast dynamic errors to types when possible and return abci responses
• Remove obsolete functions in transaction::checks

Testing

ChecTx is now running the same logic as the rest of the app, which reduces the need for extra tests for the mempool service.

Tests were added for the following scenarios:

1. A valid transaction makes it into the mempool.
2. A transaction with a nonce ahead of the current one makes it into the mempool.

More test coverage is desirable but left to a future PR.

Metrics

Removed (at present, we no longer have direct access to these)

• CHECK_TX_REMOVED_FAILED_STATELESS
• CHECK_TX_REMOVED_STALE_NONCE
• CHECK_TX_REMOVED_ACCOUNT_BALANCE
• CHECK_TX_DURATION_SECONDS with the following labels:
  • CHECK_TX_STAGE => "stateless check"
  • CHECK_TX_STAGE => "nonce check"
  • CHECK_TX_STAGE => "chain id check"
  • CHECK_TX_STAGE => "balance check"

Added (this is a coarser metrics that effectively encapsulates those above):

• CHECK_TX_REMOVED_SPECULATIVE_DELIVER_TX
• CHECK_TX_DURATION_SECONDS label:

Renamed (describes better what happened and contrasting to the speculative metric above):

• CHECK_TX_REMOVED_FAILED_EXECUTION to CHECK_TX_REMOVED_FAILED_DELIVER_TX

Breaking Changelist

• This is a breaking change because two different versions of sequencer will end up with different CheckTx results.

Related Issues

Closes #1340